Remove num-integer dependency #1037

pitdicker · 2023-04-25T14:12:07Z

Some more uses for div_euclid and rem_euclid. These were basically the only reason for the num-integer dependency, which is now no longer necessary.

I have run the benchmarks numerous times, and also tried creating optimized combined div_rem_euclid-functions. LLVM should be able to combine the div and rem into a single instruction . But the difference seems to be nothing but noise.

     Running benches/chrono.rs (target/release/deps/chrono-d6d355f24635cdec)
bench_datetime_parse_from_rfc2822
                        time:   [195.32 ns 196.89 ns 198.69 ns]
                        change: [-0.8068% +0.2830% +1.4734%] (p = 0.64 > 0.05)
                        No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

bench_datetime_parse_from_rfc3339
                        time:   [168.97 ns 169.93 ns 170.86 ns]
                        change: [-7.2979% -6.5254% -5.7402%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  6 (6.00%) high mild

bench_datetime_from_str time:   [279.07 ns 280.92 ns 283.04 ns]
                        change: [-2.6836% -1.7437% -0.8206%] (p = 0.00 < 0.05)
                        Change within noise threshold.

bench_datetime_to_rfc2822
                        time:   [97.445 ns 97.840 ns 98.286 ns]
                        change: [-6.4046% -5.6356% -4.8322%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  3 (3.00%) high mild
  5 (5.00%) high severe

bench_datetime_to_rfc3339
                        time:   [174.69 ns 176.00 ns 177.14 ns]
                        change: [-1.6935% -0.8501% -0.0899%] (p = 0.04 < 0.05)
                        Change within noise threshold.
Found 5 outliers among 100 measurements (5.00%)
  3 (3.00%) low mild
  2 (2.00%) high mild

bench_year_flags_from_year
                        time:   [0.0000 ps 0.0000 ps 0.0000 ps]
                        change: [-29.311% +28.074% +138.77%] (p = 0.47 > 0.05)
                        No change in performance detected.
Found 12 outliers among 100 measurements (12.00%)
  3 (3.00%) high mild
  9 (9.00%) high severe

num_days_from_ce/new/1  time:   [5.2447 ns 5.2753 ns 5.3099 ns]
                        change: [-1.7032% -1.0711% -0.3952%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe
num_days_from_ce/classic/1
                        time:   [1.6623 ns 1.6662 ns 1.6704 ns]
                        change: [-3.3013% -2.6817% -2.0427%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) high mild
  5 (5.00%) high severe
num_days_from_ce/new/500
                        time:   [5.4324 ns 5.4602 ns 5.4864 ns]
                        change: [+0.6683% +1.2929% +1.9432%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild
num_days_from_ce/classic/500
                        time:   [1.7459 ns 1.7574 ns 1.7708 ns]
                        change: [+3.3119% +4.2201% +5.0321%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild
num_days_from_ce/new/2000
                        time:   [5.4078 ns 5.4249 ns 5.4425 ns]
                        change: [-0.2753% +0.5890% +1.4985%] (p = 0.19 > 0.05)
                        No change in performance detected.
Found 7 outliers among 100 measurements (7.00%)
  5 (5.00%) high mild
  2 (2.00%) high severe
num_days_from_ce/classic/2000
                        time:   [1.7171 ns 1.7235 ns 1.7304 ns]
                        change: [+0.6090% +1.1668% +1.7379%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 4 outliers among 100 measurements (4.00%)
  3 (3.00%) high mild
  1 (1.00%) high severe
num_days_from_ce/new/2019
                        time:   [5.3835 ns 5.4044 ns 5.4270 ns]
                        change: [-3.5537% -2.5228% -1.5619%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  2 (2.00%) low mild
  3 (3.00%) high mild
  1 (1.00%) high severe
num_days_from_ce/classic/2019
                        time:   [1.7406 ns 1.7523 ns 1.7641 ns]
                        change: [-2.7985% -1.8732% -0.9854%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild

bench_get_local_time    time:   [107.79 ns 108.27 ns 108.78 ns]
                        change: [+5.1082% +5.8630% +6.5985%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

src/naive/time/mod.rs

jtmoon79 · 2023-04-26T05:51:08Z

AFAICT this will conflict with merged PR #1004

pitdicker · 2023-04-26T06:12:37Z

Good call, I haven't look enough at the 0.5 branch.

jtmoon79 · 2023-04-26T06:16:19Z

Good call, I haven't look enough at the 0.5 branch.

IMHO any new work should go toward that branch. More work in 0.4 branch means a more difficult transition. Relates to #970 (comment)

djc · 2023-04-26T13:34:05Z

IMHO any new work should go toward that branch. More work in 0.4 branch means a more difficult transition.

No! Any work that doesn't break API compatibility should go on 0.4.x so we can get it to users sooner.

pitdicker · 2023-04-26T14:42:32Z

After reading the issues it seems like 0.5 would require quite some design and work to do first!
I am fine with targeting the 0.4.x branch, and helping to merge it with main if that helps.

djc

Awesome, thanks!

djc · 2023-05-11T12:58:03Z

src/naive/date.rs

+fn div_mod_floor(val: i32, div: i32) -> (i32, i32) {
+    (val.div_euclid(div), val.rem_euclid(div))
+}


Style nit: small functions like this should go near the bottom of the module, not near the top.

That makes sense, I have moved to it just before the tests and serialization modules.

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [chrono](https://github.com/chronotope/chrono) | dependencies | patch | `0.4.24` -> `0.4.26` | --- ### Release Notes <details> <summary>chronotope/chrono</summary> ### [`v0.4.26`](https://github.com/chronotope/chrono/releases/tag/v0.4.26): 0.4.26 [Compare Source](chronotope/chrono@v0.4.25...v0.4.26) The changes from [#807](chronotope/chrono#807) we merged for 0.4.25 unfortunately restricted parsing in a way that was incompatible with earlier 0.4.x releases. We reverted this in [#1113](chronotope/chrono#1113). A small amount of other changes were merged since. - Update README ([#1111](chronotope/chrono#1111), thanks to [@pitdicker](https://github.com/pitdicker)) - Revert backport of [#807](chronotope/chrono#807) ([#1113](chronotope/chrono#1113), thanks to [@pitdicker](https://github.com/pitdicker)) - Update to 2021 edition ([#1109](chronotope/chrono#1109), thanks to [@tottoto](https://github.com/tottoto)) - Fix `DurationRound` panics from issue [#1010](chronotope/chrono#1010) ([#1093](chronotope/chrono#1093), thanks to [@pitdicker](https://github.com/pitdicker)) - tests: date path consolidate (branch 0.4.x) ([#1090](chronotope/chrono#1090), thanks to [@jtmoon79](https://github.com/jtmoon79)) - Parse tests nanosecond bare dot (branch 0.4.x) ([#1098](chronotope/chrono#1098), thanks to [@jtmoon79](https://github.com/jtmoon79)) - yamllint cleanup lint.yml test.yml ([#1102](chronotope/chrono#1102), thanks to [@jtmoon79](https://github.com/jtmoon79)) - Remove num-iter dependency ([#1107](chronotope/chrono#1107), thanks to [@tottoto](https://github.com/tottoto)) Thanks on behalf of the chrono team ([@djc](https://github.com/djc) and [@esheppa](https://github.com/esheppa)) to all contributors! ### [`v0.4.25`](https://github.com/chronotope/chrono/releases/tag/v0.4.25): 0.4.25 [Compare Source](chronotope/chrono@v0.4.24...v0.4.25) Time for another maintenance release. This release bumps the MSRV to 1.56; given MSRV bumps in chrono's dependencies (notably for syn 2), we felt that it no longer made sense to support any older versions. Feedback welcome in our issue tracker! ##### Additions - Bump the MSRV to 1.56 ([#1053](chronotope/chrono#1053)) - Apply comments from MSRV bump ([#1026](chronotope/chrono#1026), thanks to [@pitdicker](https://github.com/pitdicker)) - Remove num-integer dependency ([#1037](chronotope/chrono#1037), thanks to [@pitdicker](https://github.com/pitdicker)) - Add `NaiveDateTime::and_utc()` method ([#952](chronotope/chrono#952), thanks to [@klnusbaum](https://github.com/klnusbaum)) - derive `Hash` for most pub types that also derive `PartialEq` ([#938](chronotope/chrono#938), thanks to [@bruceg](https://github.com/bruceg)) - Add `parse_and_remainder()` methods ([#1011](chronotope/chrono#1011), thanks to [@pitdicker](https://github.com/pitdicker)) - Add `DateTime::fix_offset()` ([#1030](chronotope/chrono#1030), thanks to [@pitdicker](https://github.com/pitdicker)) - Add `#[track_caller]` to `LocalResult::unwrap` ([#1046](chronotope/chrono#1046), thanks to [@pitdicker](https://github.com/pitdicker)) - Add `#[must_use]` to some methods ([#1007](chronotope/chrono#1007), thanks to [@aceArt-GmbH](https://github.com/aceArt-GmbH)) - Implement `PartialOrd` for `Month` ([#999](chronotope/chrono#999), thanks to [@Munksgaard](https://github.com/Munksgaard)) - Add `impl From<NaiveDateTime> for NaiveDate` ([#1012](chronotope/chrono#1012), thanks to [@pezcore](https://github.com/pezcore)) - Extract timezone info from tzdata file on Android ([#978](chronotope/chrono#978), thanks to [@RumovZ](https://github.com/RumovZ)) ##### Fixes - Prevent string slicing inside char boundaries ([#1024](chronotope/chrono#1024), thanks to [@pitdicker](https://github.com/pitdicker)) - fix IsoWeek so that its flags are always correct ([#991](chronotope/chrono#991), thanks to [@moshevds](https://github.com/moshevds)) - Fix out-of-range panic in `NaiveWeek::last_day` ([#1070](chronotope/chrono#1070), thanks to [@pitdicker](https://github.com/pitdicker)) - Use correct offset in conversion from `Local` to `FixedOffset` ([#1041](chronotope/chrono#1041), thanks to [@pitdicker](https://github.com/pitdicker)) - Fix military timezones in RFC 2822 parsing ([#1013](chronotope/chrono#1013), thanks to [@pitdicker](https://github.com/pitdicker)) - Guard against overflow in NaiveDate::with_\*0 methods ([#1023](chronotope/chrono#1023), thanks to [@pitdicker](https://github.com/pitdicker)) - Fix panic in from_num_days_from_ce_opt ([#1052](chronotope/chrono#1052), thanks to [@pitdicker](https://github.com/pitdicker)) ##### Refactoring - Rely on std for getting local time offset ([#1072](chronotope/chrono#1072), thanks to [@pitdicker](https://github.com/pitdicker)) - Make functions in internals const ([#1043](chronotope/chrono#1043), thanks to [@pitdicker](https://github.com/pitdicker)) - Refactor windows module in `Local` ([#992](chronotope/chrono#992), thanks to [@nekevss](https://github.com/nekevss)) - Simplify from_timestamp_millis, from_timestamp_micros ([#1032](chronotope/chrono#1032), thanks to [@pitdicker](https://github.com/pitdicker)) - Backport [#983](chronotope/chrono#983) and [#1000](chronotope/chrono#1000) ([#1063](chronotope/chrono#1063), thanks to [@pitdicker](https://github.com/pitdicker)) ##### Documentation - Backport documentation improvements ([#1066](chronotope/chrono#1066), thanks to [@pitdicker](https://github.com/pitdicker)) - Add documentation for %Z quirk ([#1051](chronotope/chrono#1051), thanks to [@campbellcole](https://github.com/campbellcole)) - Add an example to Weekday ([#1019](chronotope/chrono#1019), thanks to [@pitdicker](https://github.com/pitdicker)) ##### Internal improvements - Gate test on `clock` feature ([#1061](chronotope/chrono#1061), thanks to [@pitdicker](https://github.com/pitdicker)) - CI: Also run tests with `--no-default-features` ([#1059](chronotope/chrono#1059), thanks to [@pitdicker](https://github.com/pitdicker)) - Prevent `bench_year_flags_from_year` from being optimized out ([#1034](chronotope/chrono#1034), thanks to [@pitdicker](https://github.com/pitdicker)) - Fix test_leap_second during DST transition ([#1064](chronotope/chrono#1064), thanks to [@pitdicker](https://github.com/pitdicker)) - Fix warnings when running tests on Windows ([#1038](chronotope/chrono#1038), thanks to [@pitdicker](https://github.com/pitdicker)) - Fix tests on AIX ([#1028](chronotope/chrono#1028), thanks to [@ecnelises](https://github.com/ecnelises)) - Fix doctest warnings, remove mention of deprecated methods from main doc ([#1081](chronotope/chrono#1081), thanks to [@pitdicker](https://github.com/pitdicker)) - Reformat `test_datetime_parse_from_str` ([#1078](chronotope/chrono#1078), thanks to [@pitdicker](https://github.com/pitdicker)) - GitHub yml shell `set -eux`, use bash ([#1103](chronotope/chrono#1103), thanks to [@jtmoon79](https://github.com/jtmoon79)) - test: explicitly set `LANG` to `c` in gnu `date` ([#1089](chronotope/chrono#1089), thanks to [@scarf005](https://github.com/scarf005)) - Switch test to `TryFrom` ([#1086](chronotope/chrono#1086), thanks to [@pitdicker](https://github.com/pitdicker)) - Add test for issue 551 ([#1020](chronotope/chrono#1020), thanks to [@pitdicker](https://github.com/pitdicker)) - RFC 2822 single-letter obsolete tests ([#1014](chronotope/chrono#1014), thanks to [@jtmoon79](https://github.com/jtmoon79)) - \[CI] Lint Windows target and documentation links ([#1062](chronotope/chrono#1062), thanks to [@pitdicker](https://github.com/pitdicker)) - add test_issue\_866 ([#1077](chronotope/chrono#1077), thanks to [@jtmoon79](https://github.com/jtmoon79)) - Remove AUTHORS metadata ([#1074](chronotope/chrono#1074)) On behalf of [@djc](https://github.com/djc) and [@esheppa](https://github.com/esheppa), thanks to all contributors! </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1909 Reviewed-by: crapStone <crapstone@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

pitdicker force-pushed the remove_num-integer branch from 267e59d to bedf451 Compare April 25, 2023 14:17

pitdicker commented Apr 25, 2023

View reviewed changes

src/naive/time/mod.rs Show resolved Hide resolved

pitdicker mentioned this pull request Apr 25, 2023

Simplify from_timestamp_millis, from_timestamp_micros #1032

Merged

pitdicker force-pushed the remove_num-integer branch from bedf451 to 9b1d519 Compare May 12, 2023 04:54

djc approved these changes May 15, 2023

View reviewed changes

Remove num-integer dependency

1c44129

pitdicker force-pushed the remove_num-integer branch from 9b1d519 to 1c44129 Compare May 15, 2023 10:32

djc merged commit e3f591f into chronotope:0.4.x May 15, 2023

pitdicker deleted the remove_num-integer branch May 15, 2023 11:27

pitdicker mentioned this pull request Jun 4, 2023

Potential overflows detected by Prusti #974

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove num-integer dependency #1037

Remove num-integer dependency #1037

pitdicker commented Apr 25, 2023

jtmoon79 commented Apr 26, 2023 •

edited

Loading

pitdicker commented Apr 26, 2023

jtmoon79 commented Apr 26, 2023 •

edited

Loading

djc commented Apr 26, 2023

pitdicker commented Apr 26, 2023

djc left a comment

djc May 11, 2023

pitdicker May 15, 2023

Remove num-integer dependency #1037

Remove num-integer dependency #1037

Conversation

pitdicker commented Apr 25, 2023

jtmoon79 commented Apr 26, 2023 • edited Loading

pitdicker commented Apr 26, 2023

jtmoon79 commented Apr 26, 2023 • edited Loading

djc commented Apr 26, 2023

pitdicker commented Apr 26, 2023

djc left a comment

Choose a reason for hiding this comment

djc May 11, 2023

Choose a reason for hiding this comment

pitdicker May 15, 2023

Choose a reason for hiding this comment

jtmoon79 commented Apr 26, 2023 •

edited

Loading

jtmoon79 commented Apr 26, 2023 •

edited

Loading